Why not use:

If libs.link_flags gets filled in properly for gettext as an external, that would be a reasonable thing to do.

It will add links for:

["libasprintf", "libgettextlib", "libgettextpo", "libgettextsrc", "libintl"]

It appears that, of packages requiring libintl, most of them require only that and not any of the others. Indeed, putting the full list in for elfutils actually caused an ODR violation as color_mode is defined in libeu.a and libgettextsrc (IIRC). In other words, using only -lintl when needed, rather than the full libs.link_flags, seems generally to be the way to go.

What's the build error you see without this?

If iconv is provided by libc and is an external, then this adds /usr as a high-priority prefix.

That sounds like it isn't a bug with libc but a bug with external packages. Shouldn't you instead check where iconv is installed and filter out system paths? spack.util.environment has is_system_path and filter_system_paths functions to handle this kind of stuff.

I need to double-check, but I'm moderately sure that using --libiconv-prefix tries to add -liconv to the library list which will fail if we're getting it from libc. The fouling of the library search path is secondary to that.

With reference to, "Shouldn't you instead check where iconv is installed and filter out system paths?" however: if "you" is, the package maintainer, the unfortunate answer to that is, "no, because nobody else does." As a release manager who configures a certain set of externals all the time, I'm forever running into packages where some aspect of the recipe: library search paths, PATH, or other environment settings gets tripped up by using externals. It's left to individual Spack users like me to trip over the problem, fix it in every package that has the problem and submit the PR. If there were a way to have the CI test recipe builds with dependencies configured as externals in addition to having them installed in the same Spack setup, that might help. Until then however, the burden falls on the first unfortunate soul to want to build against that dependency as an external and gets to watch everything explode.

By "you" I mean the person writing or maintaining the package.py build recipes. From your initial reply, it sounded like instead of checking whether iconv is libc, we should instead check to see if iconv is installed in a system directory where many other things are installed.

We automatically filter system deps most of the time (anything automatically added to the compiler wrappers, for example). It's hard to automatically filter them the rest of the time though. The best we can do automatically would be to ensure that all link/include flags are sorted with system locations last. Can't remember if @scheibelp or @becker33 or @haampie ever managed to get that working, or who was working on that. I agree that it's annoying to have to do this on a package-by-package basis.

By "you" I mean the person writing or maintaining the package.py build recipes. From your initial reply, it sounded like instead of checking whether iconv is libc, we should instead check to see if iconv is installed in a system directory where many other things are installed.

No, sorry.

We automatically filter system deps most of the time (anything automatically added to the compiler wrappers, for example). It's hard to automatically filter them the rest of the time though. The best we can do automatically would be to ensure that all link/include flags are sorted with system locations last. Can't remember if @scheibelp or @becker33 or @haampie ever managed to get that working, or who was working on that. I agree that it's annoying to have to do this on a package-by-package basis.

We already have environment utilities provided by me to do this:

It's already used by default.

@greenc-FNAL I'll note that our compiler wrappers reorder -L etc. so even if the configure script introduces a mixed order of system/custom paths, our wrapper should disentangle that. It would be useful to see an example failed installation and specifically the corresponding modified compiler invocations you can see in spack-cc..out if you run the install with -d like spack -d install ....

Relevant to https://github.com/spack/spack/pull/35450/files#r1164714416

It appears this is a duplicate package: ideally this could be split off into another PR (which removes one of the duplicates and updates dependencies accordingly).

Overall looking good, with a couple of typos. General notes:

• In several cases this just swaps out an explicit dep on libiconv for iconv virtual, which seems like a good thing to do in terms of linking consistency (although somewhat a separate concept to the core issue that this PR tackles)

I believe this is necessary for consistency: if libc is selected as the required provider for iconv, then packages in the DAG with explicit dependencies on libiconv could cause problems.

• I'm wondering if libintl should be treated like iconv a virtual that may be provided by libc. I think that is the case but it is not modeled that way, so the libiconv/libintl handling differs

I think this was deliberately not done because gettext may be required for non-libintl reasons. It is perfectly reasonable to use libc for libintl even in the presence of gettext in the DAG.

As a side note, it would be nice to add a configure_iconv_prefix_arg method to all implementations of the iconv virtual, which would generally avoid repetition of the following code block:

            if spec["iconv"].name == "libc":
                args.append("--without-libiconv-prefix")
            elif not is_system_path(spec["iconv"].prefix):
                args.append("--with-libiconv-prefix={p}".format(p=spec["iconv"].prefix))

That doesn't have to be done in this PR, but I'll be considering how to refactor this.

This is the "modern" way for the autotools m4 macros to handle this, but it is not the case for older autotools versions. In other words, the implementations of configure_iconv_prefix_arg() would have to consider autotools versions. Even that may not be comprehensive due to package-specific implementations of the handling, but I haven't done a survey.

This drops optimization flags :(

I have looked at the configure and mono does not seem to use configure arguments --without-libiconv-prefix or --with-libiconv-prefix. When it searches for libiconv it uses default flags. If we want to change the path for libiconv, we have to use CPPFLAGS or LDFLAGS environment variables.